Skip to content

make ind functions consistent in gauge removal#221

Open
lkdvos wants to merge 1 commit intomainfrom
ld-fix
Open

make ind functions consistent in gauge removal#221
lkdvos wants to merge 1 commit intomainfrom
ld-fix

Conversation

@lkdvos
Copy link
Copy Markdown
Member

@lkdvos lkdvos commented Apr 28, 2026

Bumped into this while trying to port this over to TensorKit:
Other parts of the code use ind = Colon() as default, and then first take indV = axes(V, 2)[ind] before doing the check of the length (length(ind) fails if ind = Colon()).
Here I just did the same.

I checked for the SVD gauge removal and was surprised that there is no ind argument there, @Jutho is this meant to be like this?

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 28, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
src/pullbacks/eig.jl 92.70% <100.00%> (ø)
src/pullbacks/eigh.jl 83.14% <100.00%> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Jutho
Copy link
Copy Markdown
Member

Jutho commented Apr 28, 2026

Let me continue on this PR, there are some more inconsistencies in how ind is treated also in the eig and eigh pullbacks. This should perhaps have been part of the previous PR, but it got merged quickly to fix the svd_full case. I will also switch over the eig and eigh pullbacks to this prepare_and_check_cotangents format, and at an ind argument in the remove_svd_gauge_dependence .

@Jutho
Copy link
Copy Markdown
Member

Jutho commented Apr 30, 2026

Upon some consideration, I think it is actually easier to remove the ind argument in the remove_x_gauge_dependence! functions. I think in these functions it is fine to assume that the V and ΔV passed in have the same number of columns. The ind is mostly an internal implementation detail that needs to be intercepted for TruncatedAlgorithm decompositions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants